Cert Advisory #CA-93:17 points out a vulnerability of xterm which can be exploited when logging is turned on. X11R6 is distributed with logging turned off. we have modified xterm to make use of the POSIX saved id where possible; otherwise, it uses setreuid() to switch back and forth between user and superuser. we provide enable() and disable() functions which swap the euid and ruid so that the running xterm can give up root and take it back. in the beginning of main, we disable, and only enable before opening the tty or utmp file (disabling immediately after). we have verified that before the log file is opened, xterm is disabled (running as the user, not root). i am appending the context diffs to the X11R6 source to Imakefile, main.c, and misc.c. compile with DEBUG2 defined to see that xterm is not root when the log file is opened. the Imakefile defines ALLOWLOGGING and SAFESETUID (our mods). xterm is installed setuid by default in X11R6 (defined in Project.tmpl), so TEST THIS AT YOUR OWN RISK. can anyone see a problem with this fix? Margarita Suarez marg@columbia.edu *** /tmp/,RCSt1019396 Tue Mar 14 12:49:21 1995 --- Imakefile Tue Mar 14 12:24:28 1995 *************** *** 27,32 **** --- 27,33 ---- -DOSMAJORVERSION=$(OSMAJORVERSION) \ -DOSMINORVERSION=$(OSMINORVERSION) MISC_DEFINES = /* -DALLOWLOGFILEEXEC */ + DEFINES = -DALLOWLOGGING -DSAFESETUID SRCS1 = button.c charproc.c cursor.c data.c input.c \ main.c menu.c misc.c screen.c scrollbar.c tabs.c \ *** /tmp/,RCSt1019396 Tue Mar 14 12:49:22 1995 --- main.c Tue Mar 14 12:42:06 1995 *************** *** 823,828 **** --- 823,832 ---- Atom wm_delete_window; + static int Xterm_euid = -1; + static int Xterm_ruid = -1; + + main (argc, argv) int argc; char **argv; *************** *** 833,838 **** --- 837,847 ---- char *base_name(); int xerror(), xioerror(); + Xterm_ruid = getuid(); + Xterm_euid = geteuid(); + + disable(); + ProgramName = argv[0]; ttydev = (char *) malloc (strlen (TTYDEV) + 1); *************** *** 1292,1298 **** return 0; #endif /* SYSV && SYSV386 */ #ifdef ATT ! if ((*pty = open ("/dev/ptmx", O_RDWR)) < 0) { return 1; } #if defined(SVR4) || defined(SYSV386) --- 1301,1307 ---- return 0; #endif /* SYSV && SYSV386 */ #ifdef ATT ! if ((*pty = enable_open ("/dev/ptmx", O_RDWR)) < 0) { return 1; } #if defined(SVR4) || defined(SYSV386) *************** *** 1304,1310 **** return 0; #else /* ATT else */ #ifdef AIXV3 ! if ((*pty = open ("/dev/ptc", O_RDWR)) < 0) { return 1; } strcpy(ttydev, ttyname(*pty)); --- 1313,1319 ---- return 0; #else /* ATT else */ #ifdef AIXV3 ! if ((*pty = enable_open ("/dev/ptc", O_RDWR)) < 0) { return 1; } strcpy(ttydev, ttyname(*pty)); *************** *** 1314,1320 **** { char *tty_name; ! tty_name = _getpty (pty, O_RDWR, 0622, 0); if (tty_name == 0) return 1; strcpy (ttydev, tty_name); --- 1323,1329 ---- { char *tty_name; ! tty_name = _getpty (pty, O_RDWR, 0622, 0); /* xxx - can't enable */ if (tty_name == 0) return 1; strcpy (ttydev, tty_name); *************** *** 1326,1332 **** char *pty_name, *getpty(); while ((pty_name = getpty()) != NULL) { ! if ((*pty = open (pty_name, O_RDWR)) >= 0) { strcpy(ptydev, pty_name); strcpy(ttydev, pty_name); ttydev[5] = 't'; --- 1335,1341 ---- char *pty_name, *getpty(); while ((pty_name = getpty()) != NULL) { ! if ((*pty = enable_open (pty_name, O_RDWR)) >= 0) { strcpy(ptydev, pty_name); strcpy(ttydev, pty_name); ttydev[5] = 't'; *************** *** 1342,1348 **** #if (defined(sgi) && OSMAJORVERSION < 4) || (defined(umips) && defined (SYSTYPE_SYSV)) struct stat fstat_buf; ! *pty = open ("/dev/ptc", O_RDWR); if (*pty < 0 || (fstat (*pty, &fstat_buf)) < 0) { return(1); } --- 1351,1357 ---- #if (defined(sgi) && OSMAJORVERSION < 4) || (defined(umips) && defined (SYSTYPE_SYSV)) struct stat fstat_buf; ! *pty = enable_open ("/dev/ptc", O_RDWR); if (*pty < 0 || (fstat (*pty, &fstat_buf)) < 0) { return(1); } *************** *** 1349,1355 **** sprintf (ttydev, "/dev/ttyq%d", minor(fstat_buf.st_rdev)); #ifndef sgi sprintf (ptydev, "/dev/ptyq%d", minor(fstat_buf.st_rdev)); ! if ((*tty = open (ttydev, O_RDWR)) < 0) { close (*pty); return(1); } --- 1358,1364 ---- sprintf (ttydev, "/dev/ttyq%d", minor(fstat_buf.st_rdev)); #ifndef sgi sprintf (ptydev, "/dev/ptyq%d", minor(fstat_buf.st_rdev)); ! if ((*tty = enable_open (ttydev, O_RDWR)) < 0) { close (*pty); return(1); } *************** *** 1381,1387 **** sprintf (ttydev, "/dev/ttyp%03d", devindex); sprintf (ptydev, "/dev/pty/%03d", devindex); ! if ((*pty = open (ptydev, O_RDWR)) >= 0) { /* We need to set things up for our next entry * into this function! */ --- 1390,1396 ---- sprintf (ttydev, "/dev/ttyp%03d", devindex); sprintf (ptydev, "/dev/pty/%03d", devindex); ! if ((*pty = enable_open (ptydev, O_RDWR)) >= 0) { /* We need to set things up for our next entry * into this function! */ *************** *** 1399,1405 **** PTYCHAR2 [devindex]; /* for next time around loop or next entry to this function */ devindex++; ! if ((*pty = open (ptydev, O_RDWR)) >= 0) { #ifdef sun /* Need to check the process group of the pty. * If it exists, then the slave pty is in use, --- 1408,1414 ---- PTYCHAR2 [devindex]; /* for next time around loop or next entry to this function */ devindex++; ! if ((*pty = enable_open (ptydev, O_RDWR)) >= 0) { #ifdef sun /* Need to check the process group of the pty. * If it exists, then the slave pty is in use, *************** *** 1635,1641 **** #endif /* LASTLOG */ #endif /* UTMP */ ! screen->uid = getuid(); screen->gid = getgid(); #ifdef SIGTTOU --- 1644,1650 ---- #endif /* LASTLOG */ #endif /* UTMP */ ! screen->uid = Xterm_ruid; screen->gid = getgid(); #ifdef SIGTTOU *************** *** 2035,2040 **** --- 2044,2050 ---- #endif /* USE_HANDSHAKE -- from near fork */ + enable(); #ifdef USE_TTY_GROUP { #include <grp.h> *************** *** 2058,2063 **** --- 2068,2074 ---- /* change protection of tty */ chmod (ttydev, 0622); #endif /* USE_TTY_GROUP */ + disable(); /* * set up the tty modes *************** *** 2371,2377 **** updwtmpx(WTMPX_FILE, &utmp); #else if (term->misc.login_shell && ! (i = open(etc_wtmp, O_WRONLY|O_APPEND)) >= 0) { write(i, (char *)&utmp, sizeof(struct utmp)); close(i); } --- 2382,2388 ---- updwtmpx(WTMPX_FILE, &utmp); #else if (term->misc.login_shell && ! (i = enable_open(etc_wtmp, O_WRONLY|O_APPEND)) >= 0) { write(i, (char *)&utmp, sizeof(struct utmp)); close(i); } *************** *** 2387,2394 **** tslot = ttyslot(); added_utmp_entry = False; { if (pw && !resource.utmpInhibit && ! (i = open(etc_utmp, O_WRONLY)) >= 0) { bzero((char *)&utmp, sizeof(struct utmp)); (void) strncpy(utmp.ut_line, ttydev + strlen("/dev/"), --- 2398,2406 ---- tslot = ttyslot(); added_utmp_entry = False; { + enable(); if (pw && !resource.utmpInhibit && ! (i = enable_open(etc_utmp, O_WRONLY)) >= 0) { bzero((char *)&utmp, sizeof(struct utmp)); (void) strncpy(utmp.ut_line, ttydev + strlen("/dev/"), *************** *** 2407,2413 **** added_utmp_entry = True; #ifdef WTMP if (term->misc.login_shell && ! (i = open(etc_wtmp, O_WRONLY|O_APPEND)) >= 0) { int status; status = write(i, (char *)&utmp, sizeof(struct utmp)); --- 2419,2426 ---- added_utmp_entry = True; #ifdef WTMP if (term->misc.login_shell && ! (i = enable_open(etc_wtmp, O_WRONLY|O_APPEND)) ! >= 0) { int status; status = write(i, (char *)&utmp, sizeof(struct utmp)); *************** *** 2416,2422 **** #endif /* WTMP */ #ifdef LASTLOG if (term->misc.login_shell && ! (i = open(etc_lastlog, O_WRONLY)) >= 0) { bzero((char *)&lastlog, sizeof (struct lastlog)); (void) strncpy(lastlog.ll_line, ttydev + --- 2429,2436 ---- #endif /* WTMP */ #ifdef LASTLOG if (term->misc.login_shell && ! (i = enable_open(etc_lastlog, O_WRONLY)) >= 0) ! { bzero((char *)&lastlog, sizeof (struct lastlog)); (void) strncpy(lastlog.ll_line, ttydev + *************** *** 2435,2441 **** #endif /* LASTLOG */ } else tslot = -tslot; ! } /* Let's pass our ttyslot to our parent so that it can * clean up after us. --- 2449,2455 ---- #endif /* LASTLOG */ } else tslot = -tslot; ! } /* Let's pass our ttyslot to our parent so that it can * clean up after us. *************** *** 2785,2791 **** updwtmpx(WTMPX_FILE, &utmp); #else /* set wtmp entry if wtmp file exists */ ! if ((fd = open(etc_wtmp, O_WRONLY | O_APPEND)) >= 0) { i = write(fd, utptr, sizeof(utmp)); i = close(fd); } --- 2799,2806 ---- updwtmpx(WTMPX_FILE, &utmp); #else /* set wtmp entry if wtmp file exists */ ! if ((fd = enable_open(etc_wtmp, O_WRONLY | O_APPEND)) >= 0) ! { i = write(fd, utptr, sizeof(utmp)); i = close(fd); } *************** *** 2801,2807 **** struct utmp utmp; if (!resource.utmpInhibit && added_utmp_entry && ! (!am_slave && tslot > 0 && (wfd = open(etc_utmp, O_WRONLY)) >= 0)){ bzero((char *)&utmp, sizeof(struct utmp)); lseek(wfd, (long)(tslot * sizeof(struct utmp)), 0); write(wfd, (char *)&utmp, sizeof(struct utmp)); --- 2816,2823 ---- struct utmp utmp; if (!resource.utmpInhibit && added_utmp_entry && ! (!am_slave && tslot > 0 && ! (wfd = enable_open(etc_utmp, O_WRONLY)) >= 0)) { bzero((char *)&utmp, sizeof(struct utmp)); lseek(wfd, (long)(tslot * sizeof(struct utmp)), 0); write(wfd, (char *)&utmp, sizeof(struct utmp)); *************** *** 2808,2814 **** close(wfd); #ifdef WTMP if (term->misc.login_shell && ! (wfd = open(etc_wtmp, O_WRONLY | O_APPEND)) >= 0) { (void) strncpy(utmp.ut_line, ttydev + sizeof("/dev"), sizeof (utmp.ut_line)); time(&utmp.ut_time); --- 2824,2830 ---- close(wfd); #ifdef WTMP if (term->misc.login_shell && ! (wfd = enable_open(etc_wtmp, O_WRONLY | O_APPEND)) >= 0) { (void) strncpy(utmp.ut_line, ttydev + sizeof("/dev"), sizeof (utmp.ut_line)); time(&utmp.ut_time); *************** *** 2827,2832 **** --- 2843,2849 ---- if (!am_slave) { /* restore ownership of tty and pty */ + enable(); chown (ttydev, 0, 0); #ifndef sgi chown (ptydev, 0, 0); *************** *** 2837,2842 **** --- 2854,2860 ---- #ifndef sgi chmod (ptydev, 0666); #endif /* !sgi */ + disable(); } exit(n); SIGNAL_RETURN; *************** *** 3062,3065 **** --- 3080,3132 ---- return killpg (pid, sig); #endif #endif + } + + disable() + { + #ifdef SAFESETUID + #ifdef _POSIX_SAVED_IDS + seteuid (Xterm_ruid); + #else + setreuid (Xterm_euid, Xterm_ruid); + #endif /* _POSIX_SAVED_IDS */ + + #ifdef DEBUG2 + fprintf (stderr, "Disabled: uid=%6d, euid=%6d, pid=%6d\n", + getuid(), geteuid(), getpid()); + #endif /* DEBUG2 */ + #endif /* SAFESETUID */ + } + + enable() + { + #ifdef SAFESETUID + #ifdef _POSIX_SAVED_IDS + seteuid (Xterm_euid); + #else + setreuid (Xterm_ruid, Xterm_euid); + #endif /* _POSIX_SAVED_IDS */ + + #ifdef DEBUG2 + fprintf (stderr, " Enabled: uid=%6d, euid=%6d, pid=%6d\n", + getuid(), geteuid(), getpid()); + #endif /* DEBUG2 */ + #endif /* SAFESETUID */ + } + + enable_open(path, flags, mode) + char *path; + int flags; + mode_t mode; + { + int ret; + + #ifdef DEBUG2 + fprintf(stderr, "opening %s\n", path); + #endif /* DEBUG2 */ + + enable(); + ret = open(path, flags, mode); + disable(); + return ret; } *** /tmp/,RCSt1019396 Tue Mar 14 12:49:24 1995 --- misc.c Tue Mar 14 12:42:20 1995 *************** *** 526,531 **** --- 526,537 ---- #endif /* SYSV */ #endif /* ALLOWLOGFILEEXEC */ + + #ifdef DEBUG2 + fprintf (stderr, " Logging: uid=%6d, euid=%6d, pid=%6d\n", + getuid(), geteuid(), getpid()); + #endif /* DEBUG2 */ + if(screen->logging || (screen->inhibit & I_LOG)) return; if(screen->logfile == NULL || *screen->logfile == 0) {